Update security.md #697
Update security.md #697
Conversation
Arbitrary commands don't need to be embedded in terraform. One can run any command or set of commands from inside a terraform variable using the notation I show in my example. The Atlantis user itself should only be able run a small set of commands. I would suggest the permissions be limited to running terraform, reading and writing only to the data-dir, and only when the source is the git repository, and the other pieces of code used to interact with the Pull Request.
Codecov Report
@@ Coverage Diff @@
## master #697 +/- ##
=======================================
Coverage 72.37% 72.37%
=======================================
Files 61 61
Lines 4658 4658
=======================================
Hits 3371 3371
Misses 1037 1037
Partials 250 250Continue to review full report at Codecov.
|
|
This wasn't intentional. We need to close this. Maybe we can limit comment args to safe characters. Thanks for reporting this though. In the future please report future security issues to security [at] runatlantis.io (see https://github.com/runatlantis/atlantis/blob/master/CONTRIBUTING.md#reporting-security-issues) |
|
Ok as long as you are aware of the issue. I'm not sure this can be avoided by limiting to just safe characters. I could in theory inject( aws destroy whatever ) and do some pretty bad damage as well. |
Remove extra quoting and instead add a backslach to each character in the extra args before appending it to the command. ex. atlantis plan -- -var key=val will result in: sh -c "atlantis plan \-\v\a\r \k\e\y\=\v\a\l" Fixes #697.
Remove extra quoting and instead add a backslach to each character in the extra args before appending it to the command. ex. atlantis plan -- -var key=val will result in: sh -c "atlantis plan \-\v\a\r \k\e\y\=\v\a\l" Fixes #697.
|
I think it fixes the issue of command injection. Some people might see it as losing functionality though. I know when I was provisioning Vault using terraform, I'd have to seed a System env var to later echo, or cat a file on the system, within the -var in order to get an LDAP password or something that I didn't want in code or shown in the merge request. Maybe a way around this is to simply point users with such a need to running custom commands via atlantis.yaml. Another option that would preserve the ability to use variables would by to escape everything except the $ |
|
Yeah I think closing the hole is a priority and then if folks like you have a special need for it then we can add in a workaround or encourage people to use the atlantis.yaml |
Arbitrary commands don't need to be embedded in terraform. One can run any command or set of commands from inside a terraform variable using the notation I show in my example. The Atlantis user itself should only be able run a small set of commands. I would suggest the permissions be limited to running terraform, reading and writing only to the data-dir, and only when the source is the git repository, and the other pieces of code used to interact with the Pull Request. I don't know how you would build this into the Altantis application, but if you would like, I can come up with an sh script to limit what I think are major vulnerabilities.